Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

전남대 FE_이도현_1주차 과제 #89

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

leedyun
Copy link

@leedyun leedyun commented Jun 29, 2024

안녕하세요.
1주차 과제 step1,2,3 완료했습니다.
storybook이라는 것을 처음 접하다보니 완벽하게 수행하지 못 한 것 같습니다.
피드백 주신다면 수정하도록 하겠습니다.
감사합니다.

tsconfig.json Outdated
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx",
"baseUrl": "src",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요. 신동리 입니다.
첫번째 리뷰네요. 앞으로 잘 해봐요 🙌🏻

baseUrl 을 '.' 로 지정하고 paths['src'] 로만 지정하면
e.g. src/components/*, src/utils/* 이런 식으로 import 가능합니다.

관련해서 다른 tsconfig 옵션들에 관해서도 어떤 역할을 하는지 함께 조사해보면 학습에 좋을거 같아요. ref. https://www.typescriptlang.org/tsconfig/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정완료했습니다. 답변 감사합니다!

/**
* Is this the principal call to action on the page?
*/
primary?: boolean;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

primary prop 을 boolean 으로 지정하기보다는 타입으로 지정하는게 확장성 측면에서 용이합니다.
예를 들면 아래와 같습니다.

아래와 같은 구조가 되면 타입이 늘어날때마다 해당하는 타입에 대응하는 스타일만 추가해주면 되기 때문입니다.

// e.g.
variant: 'primary' | 'secondary'

import React from 'react';
import './button.css';

interface ButtonProps {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

html button element 의 기본 properties 를 상속하는 방식으로 ButtonProps 를 구현하는게 확장성을 고려했을 때 좀 더 유려합니다.

예를 들면, 아래와 같습니다.

// e.g.
interface ButtonProps extends ComponentProps<'button'> {
}

@@ -0,0 +1,39 @@
.goods-item {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goods-item 만 따로 css 파일을 두신 이유가 있을까요 ?

<header>
<div className="storybook-header">
<div>
<svg width="32" height="32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

svg 파일은 react component 로 관리하는게 좋습니다.

// e.g.
export default function Add({ size, ...props }: SVGProps<SVGSVGElement>) {
  return (
    <svg
      width={size}
      height={size}
      viewBox="0 0 24 24"
      fill="none"
      xmlns="http://www.w3.org/2000/svg"
      {...props}
    >
      <g id="add-square-02" opacity="0.9">
        <path
          id="Icon"
          d="M15.375 11.9995H12M12 11.9995H8.625M12 11.9995V15.3745M12 11.9995L12 8.62445M21 12C21 16.9706 16.9706 21 12 21C7.02944 21 3 16.9706 3 12C3 7.02944 7.02944 3 12 3C16.9706 3 21 7.02944 21 12Z"
          stroke="currentColor"
          strokeLinecap="round"
        />
      </g>
    </svg>
  );
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import React from 'react';
import './Input.css';

export interface InputProps extends React.InputHTMLAttributes<HTMLInputElement> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface InputProps extends React.InputHTMLAttributes<HTMLInputElement> {
export interface InputProps extends ComponentProps<'input'> {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants